-
Notifications
You must be signed in to change notification settings - Fork 252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(2911): add validations on resolvers for federation #2916
feat(2911): add validations on resolvers for federation #2916
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2916 +/- ##
==========================================
+ Coverage 87.84% 87.85% +0.01%
==========================================
Files 263 263
Lines 26252 26301 +49
==========================================
+ Hits 23061 23108 +47
- Misses 3191 3193 +2 ☔ View full report in Codecov by Sentry. |
Action required: PR inactive for 5 days. |
Resolver::Http(http) => { | ||
Valid::from_iter(http.query.iter(), |query| { | ||
let mustache = Mustache::parse(&query.value); | ||
validate_expressions(type_name, config_module, mustache.segments().iter()) | ||
}) | ||
.and_then(|_| { | ||
let mustache = Mustache::parse(&http.path); | ||
validate_expressions(type_name, config_module, mustache.segments().iter()) | ||
}) | ||
.and(validate_iter( | ||
config_module, | ||
type_name, | ||
http.batch_key.iter(), | ||
)) | ||
.and(compile_http( | ||
config_module, | ||
http, | ||
// inner resolver should resolve only single instance of type, not | ||
// a list | ||
false, | ||
)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to parse templates and extract keys again because it's already done in the src/core/config/transformer/subgraph.rs
transformer. Also there is handler to http resolver only.
Consider making changes there the way that after Keys are calculated we add additional validation step that will check that entries are part of the currently processed type
|
||
```graphql @config | ||
schema | ||
@server(port: 8000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@server(port: 8000) | |
@server(port: 8000, enableFederation: true) |
It's required now to enable the subgraph functionality
Action required: PR inactive for 5 days. |
PR closed after 10 days of inactivity. |
Summary:
Briefly describe the changes made in this PR.
Issue Reference(s):
/claim #2911
Fixes #2911
Build & Testing:
cargo test
successfully../lint.sh --mode=fix
to fix all linting issues raised by./lint.sh --mode=check
.Checklist:
<type>(<optional scope>): <title>